solution #1588
solution #1588
Conversation
src/api/todos.ts
Outdated
| return client.get<Todo[]>(`/todos?userId=${USER_ID}`); | ||
| }; | ||
|
|
||
| // Add more methods here |
src/App.tsx
Outdated
| } | ||
|
|
||
| function handleUpdateAllTodos() { |
There was a problem hiding this comment.
Only the last todo has loader when I click toggle all:
src/App.tsx
Outdated
| return null; | ||
| }); | ||
|
|
||
| const updatedTodo = await Promise.all(updatedTodos); |
There was a problem hiding this comment.
| const updatedTodo = await Promise.all(updatedTodos); | |
| const updatedTodos = await Promise.all(updatedTodos); |
But Promise.all will be rejected if at least one todo update was failed. You should handle result for each todo separately
src/App.tsx
Outdated
| const [itemEditingId, setItemEditingId] = useState<number | null>(null); | ||
| const [newTitle, setNewTitle] = useState<string>(''); | ||
| const [todosLength, setTodosLength] = useState<number>(0); | ||
| const [allTodos, setAllTodos] = useState<Todo[]>([]); |
There was a problem hiding this comment.
You don't need todos and allTodos states. Keep only one (name todos, but store all todos there) state and compute filtered todos based on other states and save filtered array into a variable
Also do filtering inside useMemo callback
src/App.tsx
Outdated
| return <UserWarning />; | ||
| const [query, setQuery] = useState<string>(''); | ||
| const [todos, setTodos] = useState<Todo[]>([]); | ||
| const [itemsLeft, setItemsLeft] = useState<number>(0); |
There was a problem hiding this comment.
General rule - do not create state if you can compute it based on other state or props values
src/App.tsx
Outdated
| } | ||
| } | ||
|
|
||
| function handleDeleteTodo(id: number) { |
There was a problem hiding this comment.
such functions are redundant - just use setDeleteTodoId everywhere instead of handleDeleteTodo
src/App.tsx
Outdated
| } | ||
|
|
||
| function handleCleanCompleted() { | ||
| setCleanCompleted(true); |
There was a problem hiding this comment.
this state seems redundant
Do something directly inside handler instead of changing state and do it in useEffect callback
src/App.tsx
Outdated
| const [isSubmiting, setIsSubmiting] = useState<boolean>(false); | ||
| const [isUpdating, setIsUpdating] = useState<number | null>(null); | ||
| const [itemEditingId, setItemEditingId] = useState<number | null>(null); | ||
| const [newTitle, setNewTitle] = useState<string>(''); |
There was a problem hiding this comment.
why do you need query, newTask and newTitle here?
For editing move state (and part of related logic, for example itemEditingId state - you can make it just boolean inside of that component) into TodoItem component
For input in header you need only one state
src/components/Footer/Footer.tsx
Outdated
| Active | ||
| </a> | ||
|
|
||
| <a |
There was a problem hiding this comment.
each link is almost the same - try to get rid of code duplication
src/components/TodoItem/TodoItem.tsx
Outdated
| inputRef.current.focus(); | ||
| onSetNewTitle(todo.title); | ||
| } | ||
| }, [itemEditingId, todo.id]); |
src/components/Footer/Footer.tsx
Outdated
| onClick={e => { | ||
| e.preventDefault(); | ||
| onSortBy(filter); | ||
| }} |
There was a problem hiding this comment.
| onClick={e => { | |
| e.preventDefault(); | |
| onSortBy(filter); | |
| }} | |
| onClick={() => onSortBy(filter)} |
DEMO LINK